Fix a problem with functions with the same name crashing the dependency graph. #1674
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ause the dependency graph to fail.
|
Good catch! Would it be possible to add a test? Likely that test just needs to use —show-dependence-graph. |
|
Looks good please add the original regression test and this should be good to merge. The equivalent PR against develop would also be useful. |
…aph in the goto-analyser
tautschnig
reviewed
Dec 20, 2017
| { | ||
| irep_idt final_id=dest_it->first; | ||
| rename_symbols_in_function(dest_it->second, final_id, macro_application); | ||
| } |
There was a problem hiding this comment.
I don't think this change is necessary, the cases of renamed functions should all be caught above.
|
Apologies for the merge while commenting. The goto-analyze-develop branch will hopefully soon be gone in favour of moving stuff into master so as long as it is fixed in #1677, all should be well. |
You couldn't possibly have known :-) |
martin-cs
added a commit
that referenced
this pull request
Jan 10, 2018
Port #1674 to develop (Fix a problem with functions with the same name crashing the dependency graph.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before this patch, two static functions with the same name but in different files would cause
goto-analyzerto fail with an assertion violation:The problem was that one of these two clashing functions had its symbol renamed (added a
$link1postfix to its name), but the corresponding instructions of the function were not getting updated accordingly (instructionts contain a field calledfunction, which is anirep_idtreferring to the function that the instruction belongs to).Pinging @martin-cs and @chrisr-diffblue for review.